Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Message view not updating when deleting messages from search #395

Merged
merged 2 commits into from
Nov 30, 2024

Conversation

avesst
Copy link
Contributor

@avesst avesst commented Nov 29, 2024

When deleting messages from search with the API, the message view were never updated, while the message and unread count was.

@axllent
Copy link
Owner

axllent commented Nov 29, 2024

Thank you, I had not noticed this before, but you're totally right!

I do not think this is the right place for this as it translates to a websocket message for every deleted message. This is OK if there is a relatively low number of messages being deleted, however someone deletes all 10,000 messages in a tag, it means every connected browser gets 10,00 individual delete messages.

I think that a better solution would be:
a) To broadcast only after the dbLastAction = time.Now() (meaning the sql has already been executed)
b) Add some logic to the broadcast that, if the len(ids) > 200, to broadcast a "prune" instead of individual messages to just tell every client to refresh their current view.

Example (directly after dbLastAction = time.Now()):

		// broadcast changes
		if len(ids) > 200 {
			websockets.Broadcast("prune", nil)
		} else {
			for _, id := range ids {
				d := struct {
					ID string
				}{ID: id}
				websockets.Broadcast("delete", d)
			}
		}

What do you think?

@avesst
Copy link
Contributor Author

avesst commented Nov 30, 2024

Yeah you are correct. I'm not really a Go developer and had like 25 minutes between two meetings and quickly tried to get it done in that time, with this being my first time actually digging into the code base. The thought occurred to me that it would be a lot of calls but I tried it with 2000 e-mails and it seemed fine-ish, I however did not ponder the effects of multiple browsers being connected as in our use case it's rare that more than one person using Mailpit at once.

I made the change as you suggested.

@axllent
Copy link
Owner

axllent commented Nov 30, 2024

Well done on finding the related code then, considering you're "not really a Go developer" 🎉

Thank you, I'll merge this in and it'll be included in the next release later this coming week (I try not release too often, so once a week is generally more than enough).

@axllent axllent merged commit 6e44691 into axllent:develop Nov 30, 2024
@axllent
Copy link
Owner

axllent commented Dec 8, 2024

This fix has been released in v1.21.6.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Dec 9, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [axllent/mailpit](https://github.com/axllent/mailpit) | patch | `v1.21.5` -> `v1.21.6` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>axllent/mailpit (axllent/mailpit)</summary>

### [`v1.21.6`](https://github.com/axllent/mailpit/blob/HEAD/CHANGELOG.md#v1216)

[Compare Source](axllent/mailpit@v1.21.5...v1.21.6)

##### Feature

-   Include Mailpit label (if set) in webhook HTTP header ([#&#8203;400](axllent/mailpit#400))
-   Add support for sending inline attachments via HTTP API ([#&#8203;399](axllent/mailpit#399))

##### Chore

-   Update caniemail database
-   Update node dependencies
-   Update Go dependencies

##### Fix

-   Message view not updating when deleting messages from search ([#&#8203;395](axllent/mailpit#395))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants